Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix memory leak while downloading large files #1707

Closed
wants to merge 2 commits into from
Closed

fix: fix memory leak while downloading large files #1707

wants to merge 2 commits into from

Conversation

hexadivine
Copy link

Implemented generator to keep RAM usage equal to the 'chunksize' over time.

Fixes #1706

Implemented generator to keep RAM usage equal to the 'chunksize' over time.
@hexadivine hexadivine requested a review from a team as a code owner February 28, 2022 04:52
@google-cla
Copy link

google-cla bot commented Feb 28, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@hexadivine hexadivine changed the title Fix : Memory leak - issue while downloading large files #1706 Fix : Memory leak - issue while downloading large files Feb 28, 2022
@parthea parthea self-assigned this Feb 28, 2022
@parthea parthea changed the title Fix : Memory leak - issue while downloading large files fix: fix memory leak while downloading large files Mar 15, 2022
"""Constructor.

Args:
fd: io.Base or file object, The stream in which to write the downloaded
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid breaking existing users, ideally we should continue support the existing behaviour as well as the new desired behaviour.

@parthea
Copy link
Contributor

parthea commented Mar 15, 2022

Hi @h3x4d1v1n3, Thanks for opening a Pull Request! I apologize for the delay in reviewing this PR. Please could you add a test and backwards compatible option?

@parthea parthea added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Mar 15, 2022
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Mar 15, 2022
@parthea parthea assigned hexadivine and unassigned parthea Mar 15, 2022
@hexadivine
Copy link
Author

For the backward compatibility I can add another function to the lattest version that will fix this issue.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leak - issue while downloading large files
3 participants